Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use Addressable::URI everywhere #492

Merged
merged 2 commits into from
Nov 28, 2013
Merged

Use Addressable::URI everywhere #492

merged 2 commits into from
Nov 28, 2013

Conversation

matthewrudy
Copy link
Contributor

I prepared this branch before I opened issue #487

I opened an Issue rather than a PR because I felt it was likely to break compatibility.
But nevertheless it appears @sferik that we, in principle, want to use Addressable::URI instead.

So here is a commit that explicitly does so.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) when pulling 97d7c68 on matthewrudy:uri-parse into 8fd5028 on sferik:master.

@matthewrudy
Copy link
Contributor Author

@sferik the problem with 7ea2f53
is that Addressable doesn't replace ::URI with its own Addressable::URI,
so the commit actually achieves nothing.

An alternative to this PR, would be to rescue a URI parse error, and return the original string?

@sferik
Copy link
Owner

sferik commented Nov 28, 2013

Cool. Thanks.

sferik added a commit that referenced this pull request Nov 28, 2013
@sferik
Merge pull request #492 from matthewrudy/uri-parse
dd994e2 
Use Addressable::URI everywhere
@sferik sferik merged commit dd994e2 into sferik:master Nov 28, 2013
@sferik sferik mentioned this pull request Nov 28, 2013
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@matthewrudy @coveralls @sferik